poc(prow): 🚢 ✴️ openshift-eng/oape-ai-e2e workflow#79009
poc(prow): 🚢 ✴️ openshift-eng/oape-ai-e2e workflow#79009swghosh wants to merge 2 commits intoopenshift:mainfrom
Conversation
in an ephemeral namespace using the oape-ai-e2e agent Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Swarup Ghosh <swghosh@redhat.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a prow test ChangesAI E2E Workflow Test
Sequence Diagram(s)sequenceDiagram
participant Prow as Prow Job
participant Image as workflow-input (image)
participant Minter as mint-gh-token step
participant GitHub as GitHub API
participant Shared as Shared Dir
participant Agent as agent-workflow step
Prow->>Image: extract `/params.env` and copy to Shared
Prow->>Minter: start mint-gh-token (mount creds)
Minter->>GitHub: POST JWT → request installation token
GitHub-->>Minter: return installation token
Minter->>Shared: write `${SHARED_DIR}/gh-token`
Prow->>Agent: start agent-workflow (mount Shared, ADC)
Agent->>Shared: read `${SHARED_DIR}/params.env` and `${SHARED_DIR}/gh-token`
Agent->>GitHub: `gh auth setup` / API usage
Agent->>Agent: run `python3.11 main.py`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: swghosh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml`:
- Around line 154-171: This presubmit uses an untrusted PR-built image
(${AGENT_WORKER_IMAGE}) while mounting live credentials
(GOOGLE_APPLICATION_CREDENTIALS, gh-token-secret and config from
shift-worker-config) and running python3.11 /app/main.py, exposing tokens to
attacker-controlled code; fix by changing the job to use a trusted, pinned image
(replace ${AGENT_WORKER_IMAGE} with a vetted registry:tag or a cluster-built
image stream) for the credentialed execution path or move this credentialed step
out of presubmit into a postsubmit/periodic job, and ensure the env entries
(EP_URL, REPO_URL, BASE_BRANCH, PYTHONUNBUFFERED) remain the same but
credentials are only injected into runs that use the trusted image.
- Around line 125-136: Replace the fixed sleep + blind jq extraction with a
readiness loop that ensures the token-minting service is responding and
returning a non-null token before creating the secret: after starting oc
port-forward (svc/gh-token-minter) and storing PF_PID, poll the endpoint
(TOKEN_RESP via curl) with a short sleep/retry loop and a reasonable timeout,
parse GH_TOKEN from TOKEN_RESP with jq and verify it is not "null" or empty,
fail the job if the timeout is reached, and only then kill PF_PID and run oc
create secret generic gh-token-secret --from-literal=GH_TOKEN="${GH_TOKEN}";
ensure you preserve set +x around the token fetch to avoid leaking secrets.
- Around line 57-58: The step is sourcing a PR-controlled file (source
"${SHARED_DIR}/params.env") while credentials are mounted, creating an
exfiltration path; remove the direct sourcing from the credentialed step and
instead load/validate workflow inputs in an unprivileged step before mounting
secrets — e.g., move reading of SHARED_DIR/params.env into the prior
non-credentialed "workflow-input" step (or parse it with a safe parser that only
reads allowed keys), and pass only explicit, validated variables to the
credentialed step via environment variables or a short-lived file that you
create after validation; reference SHARED_DIR and params.env to find and replace
the source call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 41c98b96-6b13-4395-8b50-848a7f417edb
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml
|
/pj-rehearse |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
1782cd8 to
9bc24e7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml (2)
88-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDo not
sourcea PR-authored file after mounting credentials.
workflow-inputis built from repo content, sosource "${SHARED_DIR}/params.env"executes arbitrary shell from the PR in the same step that mounts live credentials. Parse only the expected keys instead of evaluating the file.Suggested hardening
- source "${SHARED_DIR}/params.env" - export EP_URL REPO_URL BASE_BRANCH + while IFS='=' read -r key value; do + case "${key}" in + EP_URL|REPO_URL|BASE_BRANCH) + export "${key}=${value}" + ;; + ''|\#*) + ;; + *) + echo "unexpected workflow param: ${key}" >&2 + exit 1 + ;; + esac + done < "${SHARED_DIR}/params.env"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml` around lines 88 - 89, Replace the insecure sourcing of PR-authored content—remove "source \"${SHARED_DIR}/params.env\""—and instead safely extract and export only the expected keys (EP_URL, REPO_URL, BASE_BRANCH) from ${SHARED_DIR}/params.env without executing it; update the step that currently references the source command to parse the file (e.g., by reading/grepping only lines for those variable names and exporting their values) so arbitrary shell in the PR cannot be evaluated when credentials are mounted.
75-79:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftKeep live credentials out of PR-built
agent-workersteps.Both credentialed steps run
from: agent-worker, which is built from the PR under test. That gives untrusted code access to the GitHub App key inmint-gh-tokenand the GCP credentials inagent-workflow. Use a trusted image for the credentialed path, or move this workflow to postsubmit/periodic.Also applies to: 120-124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml` around lines 75 - 79, The credentialed steps currently use "from: agent-worker" (which is built from the PR under test) and thus expose the GitHub App key (credential name openshift-app-platform-shift-github-bot, mount_path /var/run/github-app) and GCP creds to untrusted PR code; update the credentialed steps (e.g., the mint-gh-token and agent-workflow steps referenced) to use a trusted image instead of "agent-worker" or move this job to a postsubmit/periodic pipeline so credentials are only mounted into a trusted, non-PR-built image; ensure the "from" value is replaced with a pinned/trusted image name or relocate the workflow accordingly and remove credential mounts from any PR-built step.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml`:
- Around line 88-89: Replace the insecure sourcing of PR-authored content—remove
"source \"${SHARED_DIR}/params.env\""—and instead safely extract and export only
the expected keys (EP_URL, REPO_URL, BASE_BRANCH) from ${SHARED_DIR}/params.env
without executing it; update the step that currently references the source
command to parse the file (e.g., by reading/grepping only lines for those
variable names and exporting their values) so arbitrary shell in the PR cannot
be evaluated when credentials are mounted.
- Around line 75-79: The credentialed steps currently use "from: agent-worker"
(which is built from the PR under test) and thus expose the GitHub App key
(credential name openshift-app-platform-shift-github-bot, mount_path
/var/run/github-app) and GCP creds to untrusted PR code; update the credentialed
steps (e.g., the mint-gh-token and agent-workflow steps referenced) to use a
trusted image instead of "agent-worker" or move this job to a
postsubmit/periodic pipeline so credentials are only mounted into a trusted,
non-PR-built image; ensure the "from" value is replaced with a pinned/trusted
image name or relocate the workflow accordingly and remove credential mounts
from any PR-built step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8f1e4b1a-d6e8-4951-a9b3-d52f50b68cce
📒 Files selected for processing (1)
ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml
9bc24e7 to
5f4d15d
Compare
|
/pj-rehearse |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
5f4d15d to
5ac4f8c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml (1)
88-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winThe
sourcecommand security issue remains unresolved.A prior review flagged this exact pattern and it was marked "✅ Addressed in commit 1782cd8", but the current code still has
source "${SHARED_DIR}/params.env"executing PR-controlled shell code while GCP ADC credentials are mounted at/var/run/gcloud-adc.Since
params.envoriginates from the PR-builtworkflow-inputimage, a malicious PR can embed shell commands in that file to exfiltrate the mounted credentials.🔐 Recommended fix: parse only expected keys
- source "${SHARED_DIR}/params.env" - export EP_URL REPO_URL BASE_BRANCH + # Load only expected workflow params without executing shell + while IFS='=' read -r key value; do + case "${key}" in + EP_URL|REPO_URL|BASE_BRANCH) + export "${key}=${value}" + ;; + ''|\#*) + # skip blank lines and comments + ;; + *) + echo "unexpected workflow param: ${key}" >&2 + exit 1 + ;; + esac + done < "${SHARED_DIR}/params.env"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml` around lines 88 - 89, Do not source the PR-controlled file directly; replace the line source "${SHARED_DIR}/params.env" with safe parsing that reads only the expected keys (EP_URL, REPO_URL, BASE_BRANCH) and validates/sanitizes their values before exporting them. Concretely, implement parsing logic that opens "${SHARED_DIR}/params.env", extracts lines matching ^(EP_URL|REPO_URL|BASE_BRANCH)=, strips surrounding quotes, rejects/escapes any characters other than a safe whitelist (e.g., alphanumerics, /:._-), and then export the three variables (EP_URL, REPO_URL, BASE_BRANCH) instead of sourcing the file. Ensure no arbitrary commands from params.env can be executed.
🧹 Nitpick comments (1)
ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml (1)
60-73: 💤 Low valueAdd defensive error handling for GitHub API responses.
The inline Python extracts
[0]['id']and['token']without validation. If the GitHub App has no installations or the API returns an error, these will raiseIndexError/KeyErrorwith no actionable context.🛡️ Suggested improvement
- INST_ID=$(curl -sf \ - -H "Authorization: Bearer ${JWT}" \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - https://api.github.com/app/installations \ - | python3 -c "import sys,json; print(json.load(sys.stdin)[0]['id'])") + INST_ID=$(curl -sf \ + -H "Authorization: Bearer ${JWT}" \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2022-11-28" \ + https://api.github.com/app/installations \ + | python3 -c "import sys,json; d=json.load(sys.stdin); print(d[0]['id']) if d else sys.exit('No installations found')")Similar pattern for the token extraction on line 68-73.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml` around lines 60 - 73, The extraction of INST_ID and TOKEN using inline Python assumes the GitHub API returned a non-empty installations list and a token key; update the two extraction steps (the INST_ID assignment and the TOKEN assignment) to validate the HTTP response and JSON structure before accessing [0]['id'] and ['token']: check curl exit/status and/or parse the response into a variable, verify JSON is valid, ensure the installations array is non-empty (or that 'id' exists) and that 'token' exists, and on any failure emit a clear error message with the raw response and exit non-zero so callers can diagnose the problem.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml`:
- Around line 88-89: Do not source the PR-controlled file directly; replace the
line source "${SHARED_DIR}/params.env" with safe parsing that reads only the
expected keys (EP_URL, REPO_URL, BASE_BRANCH) and validates/sanitizes their
values before exporting them. Concretely, implement parsing logic that opens
"${SHARED_DIR}/params.env", extracts lines matching
^(EP_URL|REPO_URL|BASE_BRANCH)=, strips surrounding quotes, rejects/escapes any
characters other than a safe whitelist (e.g., alphanumerics, /:._-), and then
export the three variables (EP_URL, REPO_URL, BASE_BRANCH) instead of sourcing
the file. Ensure no arbitrary commands from params.env can be executed.
---
Nitpick comments:
In
`@ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml`:
- Around line 60-73: The extraction of INST_ID and TOKEN using inline Python
assumes the GitHub API returned a non-empty installations list and a token key;
update the two extraction steps (the INST_ID assignment and the TOKEN
assignment) to validate the HTTP response and JSON structure before accessing
[0]['id'] and ['token']: check curl exit/status and/or parse the response into a
variable, verify JSON is valid, ensure the installations array is non-empty (or
that 'id' exists) and that 'token' exists, and on any failure emit a clear error
message with the raw response and exit non-zero so callers can diagnose the
problem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6f158eca-249d-493a-90bd-0159e3c6fddc
📒 Files selected for processing (1)
ci-operator/config/openshift-eng/oape-ai-e2e/openshift-eng-oape-ai-e2e-main.yaml
|
/pj-rehearse |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
…mespace Signed-off-by: Swarup Ghosh <swghosh@redhat.com>
5ac4f8c to
3e8ff56
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/test owners |
|
/pj-rehearse ack |
|
@swghosh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/label tide/merge-method-squash |
|
@swghosh: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Run a Claude agent orchestrated by Prow Job structured as a pre-submit.
The expected trigger is through a PR on openshift-eng/oape-ai-e2e that updates the contents of
prow-workflow/input.Dockerfilefile.Details
Changes to openshift-eng/oape-ai-e2e CI configuration
This PR adds a ci-operator image build and test to the openshift-eng/oape-ai-e2e CI config so a Claude-based agent (run from repository openshift-eng/oape-ai-e2e) can be executed automatically when files under prow-workflow/ change.
What this modifies practically
Behavioral intent
Development/context notes
Files changed (high level)
Lines changed: +87 / -1. Estimated review effort: High.